[Renesas RX72N CCRX] Move flash functions to RAM for flash operation#689
[Renesas RX72N CCRX] Move flash functions to RAM for flash operation#689miyazakh wants to merge 2 commits intowolfSSL:masterfrom
Conversation
kojiws
left a comment
kojiws
left a comment
There was a problem hiding this comment.
Every part is good for me.
I have just one confirmation below.
There was a problem hiding this comment.
Pull request overview
This pull request implements support for executing flash write/erase operations from RAM on the Renesas RX72N platform using the CCRX compiler. The changes enable proper flash programming by moving critical flash operation functions to RAM execution, aligning flash writes to 128-byte boundaries as required by the hardware, and reducing dependency on standard library functions.
Changes:
- Added CCRX pragma directives to place flash-related functions in RAM (FRAM section) with automatic ROM-to-RAM copying at initialization
- Modified
hal_flash_write()to handle 128-byte aligned block writes with read-modify-write logic for partial blocks - Added flash error checking with new
flash_err_tenum andflash_check_error()function - Removed standard library dependencies for CCRX builds where possible
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/update_flash.c | Added CCRX pragma section directives to place update functions in FRAM section |
| src/string.c | Removed CCRX exclusions and added FRAM pragma for memcpy function (with missing closing directive) |
| src/libwolfboot.c | Added FRAM section pragma directives for partition/trailer management functions |
| src/boot_renesas.c | Added debug verify_flash_write function (unused) and debug printf |
| hal/renesas-rx.h | Added flash_err_t enum for flash error codes |
| hal/renesas-rx.c | Implemented copyfuncs() for ROM-to-RAM copy, flash_check_error(), and rewrote hal_flash_write() for 128-byte alignment; removed unused variables |
| IDE configuration files | Updated linker configuration to map PFRAM to RPFRAM sections, added compiler options, and configured standard library usage |
| app_RenesasRX01 files | Added debug functions, updated partition display logic, and linked string.c |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| static void verify_flash_write(uint32_t addr, int len) | ||
| { | ||
| uint8_t *p = (uint8_t *)addr; | ||
| int i; | ||
| printf("verify addr=0x%08x: ", addr); | ||
| for (i = 0; i < len && i < 8; i++) { | ||
| printf("%02x ", p[i]); | ||
| } | ||
| printf("\n"); | ||
| } |
There was a problem hiding this comment.
This function is defined but never called in the codebase. If it's intended for debugging purposes, consider either removing it or using it in appropriate places. If it's meant to be kept for future use, add a comment explaining its purpose.
| #ifdef WOLFBOOT_DEBUG_PARTION | ||
| static void verify_flash_write(uint32_t addr, int len) | ||
| { | ||
| uint8_t *p = (uint8_t *)addr; | ||
| int i; | ||
| printf("verify addr=0x%08x: ", addr); | ||
| for (i = 0; i < len && i < 8; i++) { | ||
| printf("%02x ", p[i]); | ||
| } | ||
| printf("\n"); | ||
| } | ||
| #endif |
There was a problem hiding this comment.
This function is defined but never called in the codebase. If it's intended for debugging purposes, consider either removing it or using it in appropriate places. If it's meant to be kept for future use, add a comment explaining its purpose.
| @@ -286,7 +284,7 @@ void RAMFUNCTION *memcpy(void *dst, const void *src, size_t n) | |||
| } | |||
There was a problem hiding this comment.
The pragma section FRAM directive is opened on line 258 but never closed. This will cause all subsequent code in the file to be placed in the FRAM section, which is likely unintended. A closing #pragma section directive should be added after the memcpy function definition, similar to how it's done in other files in this PR.
| } | |
| } | |
| #ifdef __CCRX__ | |
| #pragma section | |
| #endif |
|
|
||
| #ifdef __CCRX__ | ||
| /* copy RAM functions from ROM to RAM */ | ||
| static void copyfuncs(void) |
There was a problem hiding this comment.
Seems like this could still just use memcpy or be a version that isn't a "RAMFUNCTION" and pass in the normal dst, src, size?
| int ret, i, chunk; | ||
| uint8_t codeblock[FLASH_FACI_CODE_BLOCK_SZ]; | ||
| int i; | ||
| uint8_t codeblock[FLASH_FACI_CODE_BLOCK_SZ] = {0}; |
There was a problem hiding this comment.
= {0} isn't portable. If you need it use memset.
| block_base = addr & ~(FLASH_FACI_CODE_BLOCK_SZ - 1); | ||
| offset = addr - block_base; | ||
|
|
||
| XMEMCPY(codeblock, (uint8_t*)block_base, FLASH_FACI_CODE_BLOCK_SZ); |
There was a problem hiding this comment.
We should be using memcpy because it uses wolfboot's string.c. Are you sure you can't build the RX72 CCRX without the stdlib??
4 participants
Move flash functions to RAM for flash operation
Changed
hal_flash_write()to align address to 128-byte boundaryRemove standard library use as much as possible